-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove the dummy cache in DocContext
; delete RenderInfo
#82018
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jyn514
added
the
T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
label
Feb 12, 2021
rust-highfive
added
the
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
label
Feb 12, 2021
jyn514
force-pushed
the
no-dummy-cache
branch
from
February 12, 2021 05:07
3c6be4e
to
e4daddd
Compare
jyn514
changed the title
Remove the dummy cache in
Remove the dummy cache in Feb 12, 2021
DocContext
DocContext
; delete RenderInfo
jyn514
force-pushed
the
no-dummy-cache
branch
from
February 12, 2021 05:10
e4daddd
to
334b40e
Compare
Looks great overall! Less code makes me happier. :3 |
GuillaumeGomez
approved these changes
Feb 12, 2021
Wanna take a look maybe @ollie27 ? |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 19, 2021
…eGomez Make `Clean` take &mut DocContext - Take `FnMut` in `rustc_trait_selection::find_auto_trait_generics` - Take `&mut DocContext` in most of `clean` - Collect the iterator in auto_trait_impls instead of iterating lazily; the lifetimes were really bad. This combined with rust-lang#82018 should hopefully help with rust-lang#82014 by allowing `cx.cache.exported_traits` to be modified in `register_res`. Previously it had to use interior mutability, which required either adding a RefCell to `cache.exported_traits` on *top* of the existing `RefCell<Cache>` or mixing reads and writes between `cx.exported_traits` and `cx.cache.exported_traits`. I don't currently have that working but I expect it to be reasonably easy to add after this.
jyn514
force-pushed
the
no-dummy-cache
branch
from
February 22, 2021 02:11
334b40e
to
725b288
Compare
jyn514
commented
Feb 22, 2021
I rebased this over #82020 so I could remove the |
This comment has been minimized.
This comment has been minimized.
jyn514
force-pushed
the
no-dummy-cache
branch
from
February 22, 2021 13:44
725b288
to
23ca02d
Compare
camelid
reviewed
Feb 22, 2021
r? @camelid |
This comment has been minimized.
This comment has been minimized.
jyn514
force-pushed
the
no-dummy-cache
branch
from
February 23, 2021 20:47
4e21660
to
e4825e9
Compare
This comment has been minimized.
This comment has been minimized.
jyn514
force-pushed
the
no-dummy-cache
branch
from
February 24, 2021 18:27
e4825e9
to
c74df9f
Compare
camelid
reviewed
Feb 25, 2021
bors
added
the
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
label
Feb 28, 2021
camelid
reviewed
Feb 28, 2021
camelid
added
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Feb 28, 2021
The same information is available everywhere; the only reason the dummy cache was needed is because it waas previously stored in three different places. This consolidates the info a bit so the cache in `DocContext` is used throughout. As a bonus, it means `renderinfo` is used much much less. - Return a `Cache` from `run_global_ctxt`, not `RenderInfo` - Remove the unused `render_info` from `run_renderer` - Remove RefCell around `inlined` - Add intra-doc links
Previously, `JsonRenderer::after_krate` called `krate.version.clone()`. The problem was it did that after the version was already moved into the cache, so it would always be None. The fix was to get the version from the cache instead.
@bors r=camelid,GuillaumeGomez |
📌 Commit be069a6 has been approved by |
bors
added
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed
S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
Mar 1, 2021
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 2, 2021
…laumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#80734 (check that first arg to `panic!()` in const is `&str`) - rust-lang#81932 (Always compile rustdoc with debug logging enabled when `download-rustc` is set) - rust-lang#82018 (Remove the dummy cache in `DocContext`; delete RenderInfo) - rust-lang#82598 (Check stability and feature attributes in rustdoc) - rust-lang#82655 (Highlight identifier span instead of whole pattern span in `unused` lint) - rust-lang#82662 (Warn about unknown doc attributes) - rust-lang#82676 (Change twice used large const table to static) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Mar 6, 2021
…laumeGomez Remove RefCell around `module_trait_cache` This builds on rust-lang#82018 and should not be merged before. ## Don't require a `DocContext` for `report_diagnostic` This is needed for the next commit, which needs mutable access to the `cx` from within the `decorate` closure. - Change `as_local_hir_id` to an associated function, since it only needs a `TyCtxt` - Change `source_span_for_markdown_range` to only take a `TyCtxt` ## Remove RefCell around module_trait_cache This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Mar 6, 2021
…laumeGomez Remove RefCell around `module_trait_cache` This builds on rust-lang#82018 and should not be merged before. ## Don't require a `DocContext` for `report_diagnostic` This is needed for the next commit, which needs mutable access to the `cx` from within the `decorate` closure. - Change `as_local_hir_id` to an associated function, since it only needs a `TyCtxt` - Change `source_span_for_markdown_range` to only take a `TyCtxt` ## Remove RefCell around module_trait_cache This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
m-ou-se
added a commit
to m-ou-se/rust
that referenced
this pull request
Mar 6, 2021
…laumeGomez Remove RefCell around `module_trait_cache` This builds on rust-lang#82018 and should not be merged before. ## Don't require a `DocContext` for `report_diagnostic` This is needed for the next commit, which needs mutable access to the `cx` from within the `decorate` closure. - Change `as_local_hir_id` to an associated function, since it only needs a `TyCtxt` - Change `source_span_for_markdown_range` to only take a `TyCtxt` ## Remove RefCell around module_trait_cache This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Mar 6, 2021
…laumeGomez Remove RefCell around `module_trait_cache` This builds on rust-lang#82018 and should not be merged before. ## Don't require a `DocContext` for `report_diagnostic` This is needed for the next commit, which needs mutable access to the `cx` from within the `decorate` closure. - Change `as_local_hir_id` to an associated function, since it only needs a `TyCtxt` - Change `source_span_for_markdown_range` to only take a `TyCtxt` ## Remove RefCell around module_trait_cache This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Mar 7, 2021
…laumeGomez Remove RefCell around `module_trait_cache` This builds on rust-lang#82018 and should not be merged before. ## Don't require a `DocContext` for `report_diagnostic` This is needed for the next commit, which needs mutable access to the `cx` from within the `decorate` closure. - Change `as_local_hir_id` to an associated function, since it only needs a `TyCtxt` - Change `source_span_for_markdown_range` to only take a `TyCtxt` ## Remove RefCell around module_trait_cache This is mostly just changing lots of functions from `&DocContext` to `&mut DocContext`.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Mar 10, 2021
Remove `masked_crates` from `clean::Crate` Previously, `masked_crates` existed both on `Cache` and on `clean::Crate`. During cache population, the `clean::Crate` version was `take`n and moved to `Cache`. This change removes the version on `clean::Crate` and instead directly mutates `Cache.masked_crates` to initialize it. This has the advantage of avoiding duplication and avoiding unnecessary allocation, as well as making the flow of information through rustdoc less confusing. The one downside I see is that `clean::utils::krate()` now uses the side effect of mutating `DocContext.cache` instead of returning the data directly, but it already mutated the `Cache` for other things (e.g., `deref_trait_did`) so it's not really new behavior. Also, `clean::utils::krate()` is only called once (and is meant to only be called once since it performs expensive and potentially destructive operations) so the mutation shouldn't be an issue. Follow-up to rust-lang#82018 (comment). cc `@jyn514`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The same information is available everywhere; the only reason the dummy
cache was needed is because it was previously stored in three different
places. This consolidates the info a bit so the cache in
DocContext
isused throughout. As a bonus, it also completely removes
RenderInfo
.Cache
fromrun_global_ctxt
, notRenderInfo
render_info
fromrun_renderer
Helps with #82014. The next step is to move the
populate()
call before thecollect_intra_doc_links
pass, which currently breaks because a) lots of the cache is populated in early passes, and b) intra_doc_links itself sets some info withregister_res
. I'm working on separate PR for that to avoid making too many big changes at once.r? @GuillaumeGomez